Skip to content

Conversation

RonnyPfannschmidt
Copy link
Member

this removes the ability to pass session/config to nodes directly, and in turn exposes a few breakages in hierarchy we implicitly allowed (but shouldn't have to begin with)

I'll work out details of deprecation and moving to this later

@bluetech

This comment has been minimized.

@RonnyPfannschmidt RonnyPfannschmidt changed the base branch from features to master February 12, 2020 21:04
@nicoddemus
Copy link
Member

@RonnyPfannschmidt do you plan to pick this up after 5.4, or before?

@RonnyPfannschmidt
Copy link
Member Author

This is 6.1 stuff

@nicoddemus
Copy link
Member

nicoddemus commented Mar 4, 2020

I meant the deprecation 😁

I'll work out details of deprecation

session: Optional["Session"] = None,
parent: Union["Node", "Session"],
fspath: Optional[py.path.local] = None,
nodeid: Optional[str] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we still allow to customize the entire node id?

I know there are items which are derived from other items, for example flake8 items which are derived from normal Python items, but those can be customized by passing a different name already.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actualy an oversight, we should also disallow it quickly for from_parent

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my main intend with this pr was the structural details, so nodeid was not instrumental to the data structure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

So the main intent is to remove, in 6.1, config, session and nodeid from the Node constructor, as I understand they are the cause of structural problems, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup

@RonnyPfannschmidt
Copy link
Member Author

closing until i pick up again to clean clutter, its linked in the Node cleanup project

@RonnyPfannschmidt RonnyPfannschmidt deleted the node-only-parent branch June 25, 2022 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: discarded

Development

Successfully merging this pull request may close these issues.

3 participants